-
Notifications
You must be signed in to change notification settings - Fork 185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Run JUnit 4 based tests using junit-vintage-engine #1452
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment then LGTM, thank you @danfaer!
@@ -161,6 +161,14 @@ final class ServiceTalkLibraryPlugin extends ServiceTalkCorePlugin { | |||
private static void configureTests(Project project) { | |||
project.configure(project) { | |||
test { | |||
useJUnitPlatform() | |||
// expected format for timeout: <number>[ns|μs|ms|s|m|h|d]) | |||
def junit5DefaultTimeout = System.getProperty("CI") ? "30s" : "10s" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure System.getProperty("CI")
will give us what we need, should we use Boolean.getBoolean("CI")
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, the CI
is not a system property, it's env variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
servicetalk/docker/docker-compose.yaml
Lines 35 to 36 in 35b4293
environment: &environment | |
- CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using System.getenv("CI")
instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps def junit5DefaultTimeout = Boolean.valueOf(System.getenv("CI") ?: "false") ? "30s" : "10s"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that looks better
Motivation:
Let developers to create new JUnit 5 tests in modules not yet converted to JUnit5
Modifications:
added dependency on
org.junit.vintage:junit-vintage-engine
Result:
JUnit 4 and JUnit 5 tests can be mixed in one module
To add new JUnit 5 tests to non converted test module, the following dependencies news to be present:
testImplementation "org.junit.jupiter:junit-jupiter-api:$junit5Version"
testImplementation "org.junit.jupiter:junit-jupiter-params:$junit5Version"
(optional if using parameterized tests)testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:$junit5Version"